Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: provide access to arbitrary interpreters #2507

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

philsc
Copy link
Contributor

@philsc philsc commented Dec 16, 2024

There are some use cases that folks want to cover here. They are
discussed in this Slack thread. The high-level summary is:

  1. Users want to run the exact same interpreter that Bazel is running
    to minimize environmental issues.
  2. It is useful to pass a target label to third-party tools like mypy
    so that they can use the correct interpreter.

This patch adds to @rickeylev's work from #2359 by adding docs
and a few integration tests.

rickeylev and others added 10 commits October 29, 2024 21:05
Run a specific interpreter:
* `bazel run @rules_python//tools/run --@rules_python//python/config_settings:python_version=3.12`

Run interpreter from a binary:
* `bazel run @rules_python//tools/run --@rules_python//tools/run:bin=//my:binary`
    $ bazel run //python/bin:repl
    $ bazel run //python/bin:repl --//python/bin:repl_dep=//python/runfiles
`@rules_python//python/bin:interpreter_src` target.

```
$ bazel run @rules_python//python/bin:interpreter --@rules_python//python/bin:interpreter_src=//path/to:bin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to find an "aha!" example for this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using the publishing twine binary? Something that exists within rules_python could do the trick.

Copy link
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is still a draft, so take my suggestions with a grain of salt :)

## Accessing the underlying interpreter

To access the interpreter that bazel manages, you can use the
`@rules_python//python/bin:interpreter` target. This is a binary target with
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: why not just python?

Suggested change
`@rules_python//python/bin:interpreter` target. This is a binary target with
`@rules_python//python/bin:python` target. This is a binary target with

@@ -0,0 +1,40 @@
load("@bazel_skylib//lib:paths.bzl", "paths")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the implementation to //python/private/interpreter.bzl and re-export here

`@rules_python//python/bin:interpreter_src` target.

```
$ bazel run @rules_python//python/bin:interpreter --@rules_python//python/bin:interpreter_src=//path/to:bin
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using the publishing twine binary? Something that exists within rules_python could do the trick.

self._run_module_test("3.11")

def test_run_module_3_12(self):
self._run_module_test("3.12")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to add a test that imports:

  • The code that is in the //python/bin:interpreter_src
  • The code that is in the transitive deps of the //python/bin:interpreter_src. Maybe depending on twine could be an option in the tests and then we could attempt importing one of its deps?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants